Skip to content

feat: support %w verb in Errorf for error wrapping - #254

Open
AdamMagued wants to merge 1 commit into
pkg:masterfrom
AdamMagued:fix/issue-244
Open

AdamMagued wants to merge 1 commit into
pkg:masterfrom
AdamMagued:fix/issue-244

Conversation

@AdamMagued

Copy link
Copy Markdown

Fixes #244

Solution

  • Add %w verb support to Errorf allowing callers to wrap errors compatible with standard errors.Unwrap and errors.Is/As.
  • Add comprehensive unit test coverage for single and multi-error wrapping, unwrapping, causes, stack traces, and formatting verbs.

Verified with container sandbox test suite.

Signed-off-by: AdamMagued <adamismailmageud@gmail.com>
@davecheney

Copy link
Copy Markdown
Member

Why not just make fundamental implement Unwrap?

@AdamMagued

Copy link
Copy Markdown
Author

There are a couple of architectural considerations around fundamental and Unwrap:

  1. Leaf errors vs. wrapped errors:
    fundamental represents a leaf error created by New(msg) containing only msg string and *stack. It holds no underlying error. If fundamental were given an Unwrap() error { return nil } method, it would be a no-op for chain traversal, as standard library errors.Is and errors.As already treat types without Unwrap (or returning nil) as terminal leaves.

  2. Storing the wrapped error:
    To support Errorf("%w", err), the wrapped error operand must be retained in memory so Unwrap() can return it. If fundamental were expanded to include an err error field, every leaf error returned by New would incur an unused error field (16 bytes on 64-bit architectures).

  3. Go 1.20 multi-error wrapping:
    Standard library fmt.Errorf supports multiple %w verbs (e.g., fmt.Errorf("%w %w", err1, err2)), returning a type that implements Unwrap() []error. In Go, a single struct cannot implement both Unwrap() error and Unwrap() []error due to conflicting method return signatures.
    Following the standard library pattern (fmt.wrapError and fmt.wrapErrors), separate types allow Errorf to return:

    • wrapError (Unwrap() error) for single %w
    • wrapErrors (Unwrap() []error) for multiple %w
    • fundamental for plain format strings without %w

That said, if you prefer to keep pkg/errors focused strictly on single-error wrapping and would rather unify single-wrap Errorf into fundamental (adding err error and Unwrap() error directly to fundamental), I am glad to adjust the implementation to match your preferred design.

Comment thread errors.go
Comment on lines +115 to +129
err := fmt.Errorf(format, args...)
if u, ok := err.(interface{ Unwrap() error }); ok {
return &wrapError{
msg: err.Error(),
err: u.Unwrap(),
stack: callers(),
}
}
if u, ok := err.(interface{ Unwrap() []error }); ok {
return &wrapErrors{
msg: err.Error(),
errs: u.Unwrap(),
stack: callers(),
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😐 A type switch would make more sense here.

Comment thread errors.go
Comment on lines +136 to +140
type wrapError struct {
msg string
err error
*stack
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why recreate &withMessage{ cause: &fundamental{ … } }?

Comment thread errors.go
if u, ok := err.(interface{ Unwrap() error }); ok {
return &wrapError{
msg: err.Error(),
err: u.Unwrap(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably check to make sure that u.Unwrap is non-nil… otherwise, we should just return a fundamental.

Comment thread errors.go
if u, ok := err.(interface{ Unwrap() []error }); ok {
return &wrapErrors{
msg: err.Error(),
errs: u.Unwrap(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, if this returns a length 1 unwrap, we could just return a wrapError and a length 0 unwrap, we could just return a fundamental.

Comment thread errors.go

type wrapErrors struct {
msg string
errs []error

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why recreate &withMessage{ cause: errors.Join( &fundamental { … }, … ) }?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Errorf should be able to wrap errors

3 participants